JS: add query js/incomplete-url-substring-sanitization#623
Conversation
ghost
self-requested a review
as a 
asger-semmle
left a comment
asger-semmle
left a comment
There was a problem hiding this comment.
LGTM
One suggestion: if the URL starts with https:// or similar, the TLD check can be omitted as the HTTP protocol should be a strong enough indicator on its own. But I'll leave it up to you if you want to add that.
I agree, but there are many benign |
|
Ping @mc-semmle for a doc review. |
ghost
mentioned this pull request
mchammer01
left a comment
mchammer01
left a comment
There was a problem hiding this comment.
@esben-semmle - documentation review completed. This looks good.
I've made a couple of minor comments on the qhelp file, and I think that a verb in the @description tag of the ql file needs to be plural. Hope this helps.
|
Thanks for your latest updates to the documentation @esben-semmle - good to go from my point of view |
3 participants
This adds a query for flagging insecure substring checks for the host of an URL.
The tricky part of the query is to recognize such substring checks precisely. I have experimented with identifying likely URL and hostname values, based on associated variable and property names, but it turns out that it is sufficently precise to recognize strings as likely URLs or host names with a regular expression. We may want to downgrade the precision or sharpen further once we see the results for all of LGTM.com.
Notice that this query excludes non-trivial position checks on result of
indexOfcalls, so it should not overlap withjs/incorrect-suffix-check.Performance and results are reasonable.